i18n support for file collections - closes #4483 #4487
Conversation
gewfy
changed the title
erezrokah
added
the
type: feature
There was a problem hiding this comment.
Thanks @gewfy.
This enables the translation UI for file collections but doesn't handle persisting the content.
Can you please share an example configuration and the resulting content files of saving a file with i18n support? How would those files be named and structured in the repo?
|
@erezrokah Ok so if I understand you correctly. Basically having a file collection configured without the actual file being there? I see that this errors now. Will see if I can fix that. We're thinking about adding |
|
@erezrokah I've limited this change to only support What I've done:
Good enough trade-off? |
There was a problem hiding this comment.
Thanks @gewfy, enabling single_file for file collections makes sense.
Added a few comments.
I think the changes will require splitting https://github.com/netlify/netlify-cms/blob/519cb2d4c2db729d2643c9116f93656b6a9dba23/packages/netlify-cms-core/src/actions/config.js#L71
- Set the defaults for the collection based on the top level config.
- Set the defaults for each file based on the collection config.
- Set the defaults for each field based on the collection/file.
| @@ -634,7 +634,7 @@ describe('config', () => { | |||
| ).toEqual({ structure: 'multiple_folders', locales: ['en', 'fr'], default_locale: 'fr' }); | |||
| }); | |||
|
|
|||
| it('should throw when i18n is set on files collection', () => { | |||
| it('should throw when i18n structure is not single_file on files collection', () => { | |||
There was a problem hiding this comment.
Can we explicitly test for an error being thrown when structure is one of multiple_folders, multiple_files?
| } | ||
|
|
||
| if (collection.has('files')) { | ||
| collection = collection.set('files', traverseFields(collection.get('files'), setI18nField)); |
There was a problem hiding this comment.
traverseFields fields expects a list of fields and not of files.
| if (collection.has('files')) { | ||
| collection = collection.set( | ||
| 'files', | ||
| traverseFields(collection.get('files'), field => field.delete(I18N)), |
There was a problem hiding this comment.
traverseFields fields expects a list of fields and not of files.
| @@ -231,9 +250,17 @@ export function applyDefaults(config) { | |||
|
|
|||
| const files = collection.get('files'); | |||
| if (files) { | |||
| if (i18n && collection.has(I18N)) { | |||
| throw new Error('i18n configuration is not supported for files collection'); | |||
| if ( | |||
There was a problem hiding this comment.
Can we extract this logic to a function, e.g. isI18nAllowed?
Also I think setting:
file:
i18n:
structure: multiple_foldersWould pass this check
| @@ -245,9 +272,11 @@ export function applyDefaults(config) { | |||
| 'fields', | |||
| traverseFields(file.get('fields'), setDefaultPublicFolder), | |||
| ); | |||
| file = setI18nDefaults(i18n, file); | |||
There was a problem hiding this comment.
files should inherit collection level configuration. I think calling setI18nDefaults this way will inherit top level configuration.
This will require re-using some of the logic from here:
https://github.com/netlify/netlify-cms/blob/519cb2d4c2db729d2643c9116f93656b6a9dba23/packages/netlify-cms-core/src/actions/config.js#L74
Bump master and some tweaks/fixes based PR comments
|
Replaced by #4634 as I couldn't push to the branch of this PR |
Summary
This adds i18n support for file collections as per #4483
Test plan
Fixed unit tests:
config > applyDefaults > i18n > should throw when i18n is set on files collectionconfig > applyDefaults > i18n > should set i18n value to translate on field when i18n=true for field in files collectionWorking in development with
i18n: trueon theSettings > Site Settingsfile collection.A picture of a cute animal (not mandatory but encouraged)
Worlds smallest horse!